Conversation
|
@dotnet/roslyn-infrastructure PTAL |
| } | ||
|
|
||
| if ($binaryLogName -ne "") { | ||
| $script:binaryLog = $true |
There was a problem hiding this comment.
Initially I required specifying both options so -binaryLog -binaryLogName "Build.binlog" but that just looked too verbose to me. Decided it was concise to allow -binaryLogName to imply -binaryLog
Several of our legs were calling `build.ps1 -binaryLog` in a row and that re-uses the binary log file name which causes an overwrite. This change allows us to specify the name and adds a warning if we do accidentally overwrite in the future.
|
|
||
| if ($binaryLog -and ($binaryLogName -eq "")) { | ||
| $binaryLogName = "Build.binlog" | ||
| $script:binaryLogName = "Build.binlog" |
There was a problem hiding this comment.
Well because you asked ...
It's because when powershell reads a variable it will check function scope, then script scope. But if you write a variable it will write it in the function scope, even if one exists at script scope. So you need to be very explicit that you're writing a script scoped variable here.
There was a problem hiding this comment.
Should the $binaryLogName just above be $script:binaryLogName as well then?
There was a problem hiding this comment.
The uses on 182 are reads. Those will fall back to script scope. Line 228 is creating a new variable $binaryLogPath which is intentionally function level scope.
Why don't we always use script: when accessing script variables and instead rely on fallback rules? I don't have a good argument for that other than that's just kinda how it's written in a lot of cases. It's like asking why we require this. for extension methods but not for normal instance methods.
azure-pipelines.yml
Outdated
| - src/Dependencies/* | ||
|
|
||
| - job: Correctness_Build_Artifacts | ||
| - job: Correctness_Build_Artifacts |
There was a problem hiding this comment.
Well that's why the job hasn't started yet ... invalid YML
|
The correctness legs have all passed. Force merging so we can unblock the rest of our active PRs. |
ghost
added this to the
* Don't overwrite binary logs Several of our legs were calling `build.ps1 -binaryLog` in a row and that re-uses the binary log file name which causes an overwrite. This change allows us to specify the name and adds a warning if we do accidentally overwrite in the future. * enable trace for diff issue * Work around NuGet static graph restore bug NuGet/Home#12373
* Don't overwrite binary logs Several of our legs were calling `build.ps1 -binaryLog` in a row and that re-uses the binary log file name which causes an overwrite. This change allows us to specify the name and adds a warning if we do accidentally overwrite in the future. * enable trace for diff issue * Work around NuGet static graph restore bug NuGet/Home#12373
6 participants
Several of our legs were calling
build.ps1 -binaryLogin a row and that re-uses the binary log file name which causes an overwrite. This change allows us to specify the name and adds a warning if we do accidentally overwrite in the future.